CLDSRV-732: Fix flakiness starting cloudserver in CI#5918
CLDSRV-732: Fix flakiness starting cloudserver in CI#5918bert-e merged 1 commit intodevelopment/9.0from
Conversation
Hello bourgoismickael,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development/9.0 #5918 +/- ##
================================================
Coverage 83.19% 83.19%
================================================
Files 188 188
Lines 12093 12093
================================================
Hits 10061 10061
Misses 2032 2032
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
12b0ced to
75be1e5
Compare
The CI fails to wait for S3 after 40s recently s3-stderr has those logs: ``` npm error code E403 npm error 403 403 Forbidden - GET https://registry.npmjs.org/nyc npm error 403 In most cases, you or one of your dependencies are requesting npm error 403 a package version that is forbidden by your security policy, or npm error 403 on a server you do not have access to. npm error A complete log of this run can be found in: /root/.npm/_logs/2025-08-26T12_00_55_746Z-debug-0.log ``` Build another image to include nyc and a command for coverage during tests
75be1e5 to
0323321
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses CI flakiness by creating a dedicated Docker image for test coverage that includes the nyc package pre-installed, eliminating runtime npm install failures that were causing 40-second timeouts.
Key changes:
- Creates a multi-stage Dockerfile with separate production and test coverage images
- Moves coverage script logic from inline bash to a dedicated shell script
- Updates all CI jobs to use the new test coverage image variant
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-test-with-coverage.sh | New script containing coverage generation logic previously embedded in docker-compose |
| Dockerfile | Adds multi-stage build with testcoverage stage that pre-installs nyc |
| .github/workflows/tests.yaml | Updates all test jobs to use testcoverage image and adds build step for it |
| .github/workflows/release.yaml | Adds build and push step for testcoverage image in release workflow |
| .github/docker/docker-compose.yaml | Removes inline coverage script now handled by dedicated image |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| kill -TERM $PID 2>/dev/null || true | ||
| wait $PID |
There was a problem hiding this comment.
There's a potential race condition where the process might exit before wait $PID is called, causing the wait to fail. Consider checking if the process is still running before waiting.
| kill -TERM $PID 2>/dev/null || true | |
| wait $PID | |
| if kill -0 $PID 2>/dev/null; then | |
| wait $PID | |
| fi |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
|
/create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
ghost
left a comment
There was a problem hiding this comment.
lgtm, I'm just not sure we need to change the release workflow here
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
|
|
||
| - name: Build and push test coverage image |
There was a problem hiding this comment.
do we really want to create a new tagged image just for the coverage?
There was a problem hiding this comment.
It's there so you can run the docker compose from the CI for a specific releases version with coverage.
There was a problem hiding this comment.
Indeed we could skip coverage outside CI when targetting a specific version.
But the image is small, there's only 1 additinal layer for nyc and a change of CMD, that's 15s build time
|
/approve |
Build failedThe build for commit did not succeed in branch w/9.1/bugfix/CLDSRV-732-nyc-flakiness The following options are set: approve, create_integration_branches |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve, create_integration_branches |
Queue build failedThe corresponding build for the queue failed:
Remove the pull request from the queue
|
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-732. Goodbye bourgoismickael. |
The CI fails to wait for S3 after 40s recently
s3-stderr has those logs:
Build another image to include nyc and a command for coverage during tests